COO-178: fix: prevent other plugin types from using dynamic tracing and troubleshooting panel configurations #498
Conversation
PeterYurkovich
requested review from
danielmellado and
JoaoBraveCoding
and removed request for
a team
|
@PeterYurkovich: This pull request references COO-178 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Hi @PeterYurkovich. Thanks for your PR. I'm waiting for a rhobs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
PeterYurkovich
force-pushed
the
coo-178-fix-tp-and-dt-validation
branch
from
af03595 to
5ce5f2f
Compare
|
/lgtm |
|
/hold |
|
/assign @simonpasquier |
PeterYurkovich
force-pushed
the
coo-178-fix-tp-and-dt-validation
branch
from
5ce5f2f to
59d3cba
Compare
pkg/apis/uiplugin/v1alpha1/types.go
Outdated
| @@ -149,6 +149,8 @@ type LokiStackReference struct { | |||
|
|
|||
| // UIPluginSpec is the specification for desired state of UIPlugin. | |||
| // | |||
| // +kubebuilder:validation:XValidation:rule="self.type == 'TroubleshootingPanel' || !has(self.troubleshootingPanel)", message="TroubleshootingPanel type is required to use Troubleshooting Panel configuration" | |||
There was a problem hiding this comment.
| // +kubebuilder:validation:XValidation:rule="self.type == 'TroubleshootingPanel' || !has(self.troubleshootingPanel)", message="TroubleshootingPanel type is required to use Troubleshooting Panel configuration" | |
| // +kubebuilder:validation:XValidation:rule="self.type == 'TroubleshootingPanel' || !has(self.troubleshootingPanel)", message="Troubleshooting panel configuration is only supported with the TroubleshootingPanel type" |
There was a problem hiding this comment.
Updated the wording of the message of all the TroubleshootingPanel validations
pkg/apis/uiplugin/v1alpha1/types.go
Outdated
| @@ -149,6 +149,8 @@ type LokiStackReference struct { | |||
|
|
|||
| // UIPluginSpec is the specification for desired state of UIPlugin. | |||
| // | |||
| // +kubebuilder:validation:XValidation:rule="self.type == 'TroubleshootingPanel' || !has(self.troubleshootingPanel)", message="TroubleshootingPanel type is required to use Troubleshooting Panel configuration" | |||
| // +kubebuilder:validation:XValidation:rule="self.type == 'DistributedTracing' || !has(self.distributedTracing)", message="DistributedTracing type is required to use Distributed Tracing configuration" | |||
There was a problem hiding this comment.
Updated the wording of the message of all the DistributedTracing validations
PeterYurkovich
force-pushed
the
coo-178-fix-tp-and-dt-validation
branch
from
59d3cba to
c652261
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: PeterYurkovich, simonpasquier The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
COO-178
This PR looks the address the comments made on #480 about fixing the configuration validations for the distributed tracing and troubleshooting panel console plugins.
These configurations are currently entirely optional for both plugins, so we don't need to enforce the configuration existing if the plugin exists. We only need to prevent other plugins from using the configuration if they don't have the associated type.